Skip to content

[codegen] Convert all jennies to use codegen.AppManifest#1265

Merged
IfSentient merged 9 commits intomainfrom
update-kind-jennies
Feb 25, 2026
Merged

[codegen] Convert all jennies to use codegen.AppManifest#1265
IfSentient merged 9 commits intomainfrom
update-kind-jennies

Conversation

@IfSentient
Copy link
Copy Markdown
Contributor

What Changed? Why?

This PR converts all jennies still using codegen.Kind over to using codegen.AppManifest, and deprecates codegen.Kind. Currently, as we continue to add jennies, they have to end up in generators that are using codegen.AppManifest, and don't always fit in that space (see the go client jenny as an example, which is in the manifest generator instead of the resource one that generates all other gotype code). This change allows us to more intelligently group jennies in the future, and move the codegen model over entirely to be AppManifest-based.

How was it tested?

Codegen tests to ensure output didn't change, and created and built a new project to make sure the project codegen was still accurate.

Where did you document your changes?

N/A - no user-facing changes.

Notes to Reviewers

@IfSentient IfSentient requested a review from a team as a code owner February 24, 2026 15:02
build/operator:
ifeq ("$(wildcard cmd/operator/Dockerfile)","cmd/operator/Dockerfile")
docker build -t $(OPERATOR_DOCKERIMAGE) -f cmd/operator/Dockerfile .
docker build -t $(OPERATOR_DOCKERIMAGE) -f cmd/operator/Dockerfile .
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by that I noticed when doing new project testing--this was four spaces instead of a tab.

Comment on lines +38 to +46
for _, version := range appManifest.Versions() {
if version.Name() != appManifest.Properties().PreferredVersion {
continue
}
for _, kind := range version.Kinds() {
tmd.Resources = append(tmd.Resources, versionedKindToKindProperties(kind, appManifest))
if appManifest.Properties().FullGroup != "" {
tmd.PluginID = strings.Split(appManifest.Properties().FullGroup, ".")[0]
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is somewhat similar to the logic for getting the tmd.Resources from the manifest that you did in app generator. Can we extract this logic and potentially pass the props to the Generate func, or create a helper func that does this? That way we only do one pass through the kinds. It does seem that there are some minute differences that could make this tricky, but just an idea.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The KindProperties stuff should probably also be overhauled in the future--for now I didn't want to change the templates in this PR.

Comment on lines +36 to +37
for _, version := range appManifest.Versions() {
for _, kind := range version.Kinds() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just an idea, but could we create an func that returns an iterator to reduce this nested loop, which you use a lot throughout this PR i.e.

Suggested change
for _, version := range appManifest.Versions() {
for _, kind := range version.Kinds() {
for version, kind := range ManifestKindVersions(appManifest))) {
func ManifestKindVersions(m AppManifest) iter.Seq2[Version, VersionedKind] {
	return func(yield func(Version, VersionedKind) bool) {
		for _, version := range m.Versions() {
			for _, kind := range version.Kinds() {
				if !yield(version, kind) {
					return
				}
			}
		}
	}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added codegen.VersionedKinds and codegen.PreferredVersionKinds using iter.Seq2. Good suggestion, I keep forgetting about the iterator stuff that go introduced, and this is a great place to use it.

Comment on lines +42 to +45
if kind.Scope != string(resource.NamespacedScope) && kind.Scope != string(resource.ClusterScope) {
return nil, fmt.Errorf("scope '%s' is invalid, must be one of: '%s', '%s'",
kind.Scope, resource.ClusterScope, resource.NamespacedScope)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit; maybe you could use a helper func for this scope check?

Comment on lines 126 to -140
@@ -134,10 +146,6 @@ func (o *operatorMainJenny) Generate(kinds ...codegen.Kind) (*codejen.File, erro
KindsAreGrouped: !o.groupByKind,
}

for _, kind := range kinds {
tmd.Resources = append(tmd.Resources, kind.Properties())
}

Copy link
Copy Markdown
Contributor

@amalavet amalavet Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can you just remove this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resources isn't used in the template, I forgot to remove it from the metadata as well, let me fix that.

Comment on lines 90 to 94
files = append(files, codejen.File{
RelativePath: filepath.Join(GetGeneratedGoTypePath(r.GroupByKind, appManifest.Properties().Group, version.Name(), kind.MachineName), fmt.Sprintf("%s_object_gen.go", strings.ToLower(kind.MachineName))),
Data: b,
From: []codejen.NamedJenny{r},
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit; just a little cleaner to follow

Suggested change
files = append(files, codejen.File{
RelativePath: filepath.Join(GetGeneratedGoTypePath(r.GroupByKind, appManifest.Properties().Group, version.Name(), kind.MachineName), fmt.Sprintf("%s_object_gen.go", strings.ToLower(kind.MachineName))),
Data: b,
From: []codejen.NamedJenny{r},
})
typePath := GetGeneratedGoTypePath(r.GroupByKind, appManifest.Properties().Group, version.Name(), kind.MachineName)
suffix := fmt.Sprintf("%s_object_gen.go", strings.ToLower(kind.MachineName))
files = append(files, codejen.File{
RelativePath: filepath.Join(typePath, suffix),
Data: b,
From: []codejen.NamedJenny{r},
})

// Any type parser should be able to parse a kind into this definition to supply
// to various common Jennies in the codegen package.
// Deprecated: use AppManifest instead
type Kind interface {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still being used in some places, any reason why not removing it across the repo?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only place left that still uses it AFAICT are the parsers, which are exported. I'll deprecate the KindParser, but given that it's an exported type, I want ot leave it present and deprecated for a few releases just in case anyone is using it.

…en.Kind, create convenience iterator functions in the codegen package for iterating through versions+kinds in order, other small fixes based on PR comments.
@amalavet amalavet self-requested a review February 25, 2026 19:46
@IfSentient IfSentient merged commit 03a4875 into main Feb 25, 2026
16 checks passed
@IfSentient IfSentient deleted the update-kind-jennies branch February 25, 2026 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants